require_field_match now defaults to true
#11067
Conversation
The default `false` for `require_field_match` is a bit odd and confusing for users, given that field names get ignored by default and every field gets highlighted if it contains terms extracted out of the query, regardless of which fields were queries. Changed the default to `true`, it can always be changed per request. Closes elastic#10627
javanna
added
>enhancement
>breaking
v2.0.0-beta1
review
:Search/Highlighting
|
+1 I'm so glad this is built in and doesn't have to happen 4 times.... |
|
@javanna I left some comment. |
| @@ -1914,8 +1879,8 @@ public void testPostingsHighlighter() throws Exception { | |||
|
|
|||
| logger.info("--> searching on _all, highlighting on field1"); | |||
| source = searchSource() | |||
| .query(termQuery("_all", "test")) | |||
| .highlight(highlight().field("field1").preTags("<xxx>").postTags("</xxx>")); | |||
| .query(termQuery("field1", "test")) | |||
There was a problem hiding this comment.
Why change the field name to "field1"?
There was a problem hiding this comment.
because if we keep _all and highlight field1 there we get no output back. I didn't rely on requireFieldMatch for this one as the postings highlighter is soon not going to support it anymore, see #11077
There was a problem hiding this comment.
I see. Then, I think we should change the log message from searching on _all to searching on field1
There was a problem hiding this comment.
thanks for the review Jun I changed this test to leave querying for _all and set require_field_match to false explicitly. Will change again when #11077, doesn't make much sense to take it into account here.
|
@clintongormley you ok with this change? |
|
looks good |
|
@javanna +1 |
clintongormley
changed the title
require_field_match now defaults to true
The default
falseforrequire_field_matchis a bit odd and confusing for users, given that field names get ignored by default and every field gets highlighted if it contains terms extracted out of the query, regardless of which fields were queried. Changed the default totrue, it can always be customized per request.Closes #10627